Dataflow: Add support for speculative taint flow. #17663
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
|
|
||
| signature int speculationLimitSig(); | ||
|
|
||
| private module AddSpeculativeTaintSteps< |
Check warning
Code scanning / CodeQL
Data flow configuration module naming Warning
| module SpeculativeFlow<DataFlow::ConfigSig Config, speculationLimitSig/0 speculationLimit> | ||
| implements DataFlow::GlobalFlowSig | ||
| { | ||
| private module Config0 implements DataFlowInternal::FullStateConfigSig { |
Check warning
Code scanning / CodeQL
Data flow configuration module naming Warning
| } | ||
| } | ||
|
|
||
| private module C implements DataFlowInternal::FullStateConfigSig { |
Check warning
Code scanning / CodeQL
Data flow configuration module naming Warning
| DataFlow::StateConfigSig Config, speculationLimitSig/0 speculationLimit> implements | ||
| DataFlow::GlobalFlowSig | ||
| { | ||
| private module Config0 implements DataFlowInternal::FullStateConfigSig { |
Check warning
Code scanning / CodeQL
Data flow configuration module naming Warning
| } | ||
| } | ||
|
|
||
| private module C implements DataFlowInternal::FullStateConfigSig { |
Check warning
Code scanning / CodeQL
Data flow configuration module naming Warning
There was a problem hiding this comment.
Seems like a really useful tool. I've done something similar before during Swift development, it was crude (necessarily as we didn't have provenance labels at the time), but helpful nevertheless.
I gather it's not (currently) possible to combine speculative taint flow with an existing flow state in the query?
Is it possible for any of the changes to affect performance when speculation is not being used?
No, I can happily report that combining the two very much is supported!
No. |
aschackmull
force-pushed
the
dataflow/speculative-flow
branch
from
605452b
to
42d35f8
Compare
|
For Ruby, I've now made some exclusions guided by the consistency check. For review please check if those are reasonable. |
csharp/ql/lib/semmle/code/csharp/dataflow/internal/TaintTrackingPrivate.qll
Show resolved
Hide resolved
csharp/ql/lib/semmle/code/csharp/dataflow/internal/TaintTrackingPrivate.qll
Show resolved
Hide resolved
| * Constructs a global taint tracking computation that also allows a given | ||
| * maximum number of speculative taint steps. | ||
| */ | ||
| module SpeculativeFlow<DataFlow::ConfigSig Config, speculationLimitSig/0 speculationLimit> |
There was a problem hiding this comment.
Perhaps it should be named SpeculativeGlobal instead?
There was a problem hiding this comment.
I'm fine with that. I'll push a rename shortly.
| * Constructs a global taint tracking computation using flow state that also | ||
| * allows a given maximum number of speculative taint steps. | ||
| */ | ||
| module SpeculativeFlowWithState< |
aschackmull
added
the
no-change-note-required
This adds support for speculative taint flow in the shared taint tracking library.
What is this?
This is a magic button (dial, really) that you can turn to calculate more taint flow in order to identify false negatives. So if you suspect a FN, e.g. if you're failing to find a flow for a CVE or you're facing zero results thinking that we might be missing some models, then try this!
How does it work?
Each language provides a huge candidate set of potential taint steps. The default set that I've implemented is simply any argument to any return value (plus potential side-effects on the
thisargument, if any) on any call for which we don't yet have an existing model or a call target within the analyzed source.The shared library will then execute the regular taint flow, but in addition it will allow speculative flow steps drawn from this candidate set up to a specified maximum number of such edges along a given path.
It will then report flow in the usual way, and the chosen speculative edges will be visible in the path explanation with the provenance label "Speculative". (In the VSCode plugin this shows up as "(step) Speculative".)
I want to try it, show me how!
It's easy, just replace the application of the
TaintTracking::Globalmodule withTaintTracking::SpeculativeGlobal. So if you e.g. havethen replace that with
The number you choose in the
speculationLimitis the limit on the number of speculative steps that can be used in a path. A higher number gives more flow, but worse performance. Expect a performance degradation factor roughly equal to the chosen limit.Testing so far, and followup work for the individual language teams
I've tested this for Java and C# with a number queries on their respective MRVA top100 with good results and reasonable performance. For the remaining languages, the candidate set of edges might need further tweaking to e.g. exclude things that happen to be calls, but which shouldn't be considered as potential taint steps. For C# I e.g. had to reduce the set to "only" include method and constructor calls, i.e. no operator nor property calls (I believe the latter is already included as read/store steps).